Skip to content

Fix escaping XPath characters & add set_values_by_xpaths - #476

Merged
iMicknl merged 1 commit into
iMicknl:mainfrom
WASDi:fix-xpath-escape
May 6, 2026
Merged

Fix escaping XPath characters & add set_values_by_xpaths#476
iMicknl merged 1 commit into
iMicknl:mainfrom
WASDi:fix-xpath-escape

Conversation

@WASDi

@WASDi WASDi commented May 3, 2026

Copy link
Copy Markdown
Contributor

This fixes XPath expressions like get_value_by_xpath("Device/Hosts/Hosts/Host[@uid=\"8258\"]/UserFriendlyName").

Also added a method set_values_by_xpaths to set multiple values in one call, as was available to get multiple values.

Updated README.md to reflect actual python version needed according to pyproject.toml.

@WASDi
WASDi requested a review from iMicknl as a code owner May 3, 2026 21:10
@iMicknl
iMicknl requested a review from Copilot May 4, 2026 13:16

@iMicknl iMicknl left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @WASDi! Are you using the library directly or via the Home Assistant by any chance?

A few remarks:

Comment thread sagemcom_api/client.py
Comment thread sagemcom_api/client.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves XPath handling in the async Sagemcom router client by adjusting how XPath strings are encoded (to support attribute selectors containing @ and "), adds a new bulk setter API for setting multiple values in one request, and updates the README to match the project’s declared minimum Python version.

Changes:

  • Route get_value_by_xpath through get_values_by_xpaths and expand XPath quoting “safe” characters to include @ and ".
  • Add set_values_by_xpaths to set multiple XPath/value pairs in a single API call (and refactor set_value_by_xpath to use it).
  • Update README minimum Python version to 3.11+ (aligned with pyproject.toml).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
sagemcom_api/client.py Adjust XPath escaping and add a bulk setter API for multi-action setValue requests.
README.md Update stated minimum Python version to match project metadata.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sagemcom_api/client.py
Comment thread sagemcom_api/client.py
Comment thread sagemcom_api/client.py
Comment thread sagemcom_api/client.py
@WASDi

WASDi commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

I am using the library directly. Thanks for making it!

@WASDi
WASDi requested a review from iMicknl May 6, 2026 16:48

@iMicknl iMicknl left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@iMicknl
iMicknl merged commit d44f9b0 into iMicknl:main May 6, 2026
9 checks passed
@iMicknl iMicknl changed the title Fix escaping XPath characters Fix escaping XPath characters & add set_values_by_xpaths May 6, 2026
@WASDi
WASDi deleted the fix-xpath-escape branch May 6, 2026 19:14
mr-miles added a commit to mr-miles/python-sagemcom-api that referenced this pull request Jul 6, 2026
TalkTalk (UK) branded F5364 does not recognise the ModelNumber attribute
requested in GetDeviceInfo, which caused an exception because the response
handler threw for any ActionError. As a result HA could not connect to the
router to retrieve useful info, even though ModelNumber is not essential.

Fixed by:
 - Lifting ActionError handling out of the low-level POST into a dedicated
   ActionErrorHandler, so callers can react appropriately (and multiple
   actions can be handled individually, addressing the previous TODO).
 - Adding a suppress_action_errors option so get_value(s)_by_xpath can
   ignore unknown-path errors and return None for missing values, while
   still raising genuine errors (auth, etc.).
 - Using that option in get_device_info's fallback so missing DeviceInfo
   attributes are tolerated.

Existing error-raising behaviour is retained for all other calls, which now
invoke ActionErrorHandler.throw_if_error explicitly.

Tests cover the suppression behaviour of get_value(s)_by_xpath, the
end-to-end get_device_info fallback for the reported F5364 case, and the
ActionErrorHandler in isolation (throw_if_error / throw_if_error_at /
from_error_description).

Rebased onto upstream main, preserving the XPath-escaping fix (iMicknl#476),
get_logs, and the speed-test additions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@iMicknl iMicknl added fix bug Something isn't working labels Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants